refactor: Replace hard code page size. #2622
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ready for review @lukebp |
vibros68
changed the title
vibros68
changed the title
vibros68
changed the title
lukebp
requested changes
Oct 2, 2021
There was a problem hiding this comment.
I'm still seeing a lot of hardcoded page sizes.
$ grep -rinI --exclude-dir={node_modules,build,bin} page_size *
src/containers/Proposal/helpers.js:33: PROPOSAL_PAGE_SIZE,
src/containers/Proposal/helpers.js:458: pageSize = PROPOSAL_PAGE_SIZE
src/containers/Proposal/User/hooks.js:24:import { PROPOSAL_PAGE_SIZE } from "src/constants";
src/containers/Proposal/User/hooks.js:84: proposalPageSize = PROPOSAL_PAGE_SIZE,
src/containers/Proposal/User/Submitted.jsx:14:const PAGE_SIZE = 20;
src/containers/Proposal/User/Submitted.jsx:60: amountOfMissingProposals > PAGE_SIZE ? PAGE_SIZE : amountOfMissingProposals;
src/containers/Proposal/Download/hooks.js:11:const TIMESTAMPS_PAGE_SIZE = 100;
src/containers/Proposal/Download/hooks.js:18: const multiPage = votesCount > TIMESTAMPS_PAGE_SIZE;
src/containers/Proposal/Download/hooks.js:25: total ? ((total * TIMESTAMPS_PAGE_SIZE) / votesCount).toFixed(0) : 0,
src/containers/Comments/Download/hooks.js:38:const TIMESTAMPS_PAGE_SIZE = 100;
src/containers/Comments/Download/hooks.js:53: const multiPage = commentsLength > TIMESTAMPS_PAGE_SIZE;
src/containers/Comments/Download/hooks.js:62: take(TIMESTAMPS_PAGE_SIZE)(commentIds),
src/containers/Comments/Download/hooks.js:63: takeRight(commentIds.length - TIMESTAMPS_PAGE_SIZE)(commentIds)
src/constants.js:102:export const PROPOSAL_PAGE_SIZE = 5;
src/constants.js:103:export const INVENTORY_PAGE_SIZE = 20;
src/components/RecordsView/RecordsView.jsx:12: PROPOSAL_PAGE_SIZE
src/components/RecordsView/RecordsView.jsx:68: pageSize = PROPOSAL_PAGE_SIZE,
src/hooks/api/useProposalsBatch.js:26: INVENTORY_PAGE_SIZE,
src/hooks/api/useProposalsBatch.js:27: PROPOSAL_PAGE_SIZE,
src/hooks/api/useProposalsBatch.js:92: proposalPageSize = PROPOSAL_PAGE_SIZE,
src/hooks/api/useProposalsBatch.js:93: inventoryPageSize = INVENTORY_PAGE_SIZE
src/hooks/api/useApprovedProposals.js:5:const PAGE_SIZE = 20;
src/hooks/api/useApprovedProposals.js:25: PAGE_SIZE
teste2e/cypress/e2e/proposal/list.js:12:const RECORDS_PAGE_SIZE = 5;
teste2e/cypress/e2e/proposal/list.js:186: cy.assertListLengthByTestId("record-title", RECORDS_PAGE_SIZE) // first records batch
teste2e/cypress/e2e/proposal/list.js:260: cy.assertListLengthByTestId("record-title", RECORDS_PAGE_SIZE).each(
|
Some of those are my mistake. But some others are I leave to be a default value. Do you want to remove those completely? |
|
Hrmm some are not related to the backend page size and I used for other purposes. Examples: Those I guess can stay for now, but all the API related ones should be removed. |
amass01
reviewed
Oct 13, 2021
| @@ -5,6 +5,9 @@ import ProposalLoader from "src/components/Proposal/ProposalLoader"; | |||
| import useQueryStringWithIndexValue from "src/hooks/utils/useQueryStringWithIndexValue"; | |||
| import { tabValues, statusByTab } from "./helpers"; | |||
| import { mapProposalsTokensByTab } from "src/containers/Proposal/helpers"; | |||
| import usePolicy from "src/hooks/api/usePolicy"; | |||
| // XXX change to AdminActionsProvider | |||
There was a problem hiding this comment.
May be it is generated automatically from code editor. I will remove it now. Thanks for your review @amass01
amass01
reviewed
Oct 13, 2021
|
Teste2e run successfully |
amass01
suggested changes
Oct 15, 2021
There was a problem hiding this comment.
I see many tests are missing, run the following command to run all tests in teminal:
yarn test:e2e:run
Also make sure you don't have any .only on one of the tests.
Please don't post the test results in the PR commit message, makes it unreadable
a separate comment is ok, see my comment here #2629 (comment) as an example -
you can add collapsable sections to github's markdown which is quite useful is such cases.
|
Full detail teste2e for #ed27150 Click to expand!
|
amass01
reviewed
Oct 18, 2021
| @@ -108,7 +108,7 @@ describe("Proposal Edit", () => { | |||
| }) => { | |||
| cy.approveProposal(censorshiprecord); | |||
| cy.visit(`record/${shortRecordToken(censorshiprecord.token)}`); | |||
| cy.wait(1000); | |||
| cy.wait(3000); | |||
There was a problem hiding this comment.
why you need to wait 3 seconds here ?
if you are waiting to some API request, then wait for it
explicitly as we do in all other tests waiting for API requests.
amass01
suggested changes
Oct 18, 2021
There was a problem hiding this comment.
- Your logs are full of warnings/errors where are they coming from ?
...
Admin proposals actions
✓ Should allow admins to approve propsoals (24456ms)
[HPM] Error occurred while trying to proxy request /comments/v1/comments from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
✓ Should allow admins to report a proposal as a spam (23389ms)
✓ Should allow admins to abandon proposals (20140ms)
✓ Should allow proposal author to authorize voting (19863ms)
[HPM] Error occurred while trying to proxy request /comments/v1/votes from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
[HPM] Error occurred while trying to proxy request /comments/v1/votes from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
✓ Should allow admins to set the billing status of an active propsoal (24456ms)
[HPM] Error occurred while trying to proxy request /comments/v1/votes from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
[HPM] Error occurred while trying to proxy request /comments/v1/votes from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
[HPM] Error occurred while trying to proxy request /comments/v1/comments from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
✓ Should allow admins to set the billing status of a closed propsoal (23878ms)
[HPM] Error occurred while trying to proxy request /comments/v1/votes from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
[HPM] Error occurred while trying to proxy request /comments/v1/votes from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
[HPM] Error occurred while trying to proxy request /comments/v1/comments from localhost:3000 to https://localhost:4443 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)
✓ Should allow admins to set the billing status of a completed propsoal (23569ms)
✓ Shouldn't allow admins set a billing status when number of billing status changes exceeds the `billingstatuschangesmax` policy (20317ms)
...
Some requests to the server is failed. It becauses my computer is weak. When it is running heavy it losts some requests |
|
About changed from 1 second to 3 seconds. It is not waitting for an api request. It is after creating a proposal and click edit button. But because my laptop is weak and while running entire the teste2e. It becomes very slow. If I let 1 sec on there. The test will not stable. Some time it is passed and some time it is not. @amass01 |
Closed
This was referenced Dec 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2604.
decred/politeia#1518 adds the page sizes to the policy replies. This
diff replace hardcoded page sizes to be the policy reply values.